Less confusing message for failed test returning Result #97586

printercu · 2022-05-31T15:53:12Z

Failure message contained a part that is related to assert_eq! matcher.
This is confusing in the cases when test fails because of Err result.

Test:

#[test]
fn test() -> Result<(), &'static str> {
    Err("some failure")?;
    assert_eq!(1, 2);
    Ok(())
}

Before:

---- tests::test stdout ----
Error: "some failure"
thread 'tests::test' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5

After:

 ---- tests::test stdout ----
Error: "some failure"
thread 'tests::test' panicked at 'the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5

Fixes #69517

Failure message contained a part that is related to `assert_eq!` matcher. This is confusing in the cases when test fails because of Err result. Test: ```rust #[test] fn test() -> Result<(), &'static str> { Err("some failure")?; assert_eq!(1, 2); Ok(()) } ``` Before: ``` ---- tests::test stdout ---- Error: "some failure" thread 'tests::test' panicked at 'assertion failed: `(left == right)` left: `1`, right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5 ``` After: ``` ---- tests::test stdout ---- Error: "some failure" thread 'tests::test' panicked at 'the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5 ```

rust-highfive · 2022-05-31T15:53:14Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

rust-highfive · 2022-05-31T15:53:16Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

ehuss · 2022-05-31T17:57:30Z

This may close #69517.

Dylan-DPC · 2022-06-02T12:55:30Z

library/test/src/lib.rs

    let code = result.report().to_i32();
-    assert_eq!(
-        code, 0,
+    assert!(


assert_eq also accepts an error message

It was assert_eq with error message. It results in confusing output, please see examples in initial comment or diff's comment

The assert_eq here doesn't help, because the message already reports the code value, if it needs to be reported at all. Instead, it harms as shown in the description, in #69517 and in #59263.

In fact the fix was already approved as #59263 but eventually landed in an outdated repository.

Mark-Simulacrum · 2022-07-31T21:03:23Z

@bors r+

bors · 2022-07-31T21:03:25Z

📌 Commit b4cc991 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

klensy · 2022-07-31T21:20:17Z

And without test and/or comment, this can be accidentally changed back.

Mark-Simulacrum · 2022-07-31T21:28:41Z

@bors r-

Good call! We can add a test pretty easily along the lines of src/test/ui/test-attrs/test-panic-abort-nocapture.rs, which runs some tests and captures the stdout in src/test/ui/test-attrs/test-panic-abort-nocapture.run.stdout.

(Obviously, we don't need the panic=abort-ness for this test).

bors · 2022-10-02T18:31:51Z

☔ The latest upstream changes (presumably #102586) made this pull request unmergeable. Please resolve the merge conflicts.

bobhy · 2022-12-18T04:28:48Z

It seems that this PR was unmergable (by the time it was accepted) and the issue was simply closed. Would a new PR allow the issue to be reopened?

The original problem is that test functions cannot report meaningful errors if they return a Err(E) from a ? operator (but can if same line uses .unwrap() instead), agreed?
That does force developers to use non-idiomatic syntax, but only in tests. That's kind of hard on our poor little brains. So, I would be glad to take a crack at a PR to fix this if it could be reviewed soonish.

metasim · 2022-12-20T19:48:19Z

@printercu @Mark-Simulacrum Would you consider re-submitting this?

printercu · 2022-12-20T21:02:34Z

@metasim #100451 made this pr obsolete.

printercu · 2022-12-20T21:18:28Z

@bobhy As I understand tests will try to print error into stderr if it is fmt::Debug (https://doc.rust-lang.org/src/std/process.rs.html#2211). I'm not sure if it's possible to display any arbitrary type in the same way.

cole-abbeduto-particle · 2023-03-17T17:06:40Z

@bobhy As I understand tests will try to print error into stderr if it is fmt::Debug (https://doc.rust-lang.org/src/std/process.rs.html#2211). I'm not sure if it's possible to display any arbitrary type in the same way.

That is true and a slight improvement. But we've also lost the line number in the process.

`unwrap`:

---- test::will_this_fail stdout ----
thread 'test::will_this_fail' panicked at 'called `Result::unwrap()` on an `Err` value: Is this a failure', /src/bin/file.rs:179:65
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

`return Err` or `?`:

---- test::will_this_fail stdout ----
Error: Is this a failure?

rust-highfive assigned m-ou-se May 31, 2022

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 31, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 31, 2022

Dylan-DPC reviewed Jun 2, 2022

View reviewed changes

JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 31, 2022

Mark-Simulacrum assigned Mark-Simulacrum and unassigned m-ou-se Jul 31, 2022

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 31, 2022

printercu closed this Oct 3, 2022

printercu deleted the patch-1 branch December 20, 2022 21:02

Less confusing message for failed test returning Result #97586

Less confusing message for failed test returning Result #97586

Uh oh!

Conversation

printercu commented May 31, 2022 • edited by JohnTitor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented May 31, 2022

Uh oh!

rust-highfive commented May 31, 2022

Uh oh!

ehuss commented May 31, 2022

Uh oh!

Dylan-DPC Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

printercu Jun 2, 2022

Choose a reason for hiding this comment

Uh oh!

ssomers Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum commented Jul 31, 2022

Uh oh!

bors commented Jul 31, 2022

Uh oh!

klensy commented Jul 31, 2022

Uh oh!

Mark-Simulacrum commented Jul 31, 2022

Uh oh!

bors commented Oct 2, 2022

Uh oh!

bobhy commented Dec 18, 2022

Uh oh!

metasim commented Dec 20, 2022

Uh oh!

printercu commented Dec 20, 2022

Uh oh!

printercu commented Dec 20, 2022

Uh oh!

cole-abbeduto-particle commented Mar 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

unwrap:

return Err or ?:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

printercu commented May 31, 2022 •

edited by JohnTitor

Loading

ssomers Jun 7, 2022 •

edited

Loading

cole-abbeduto-particle commented Mar 17, 2023 •

edited

Loading

`unwrap`:

`return Err` or `?`: